Skip to content

ENH: Add Heart-Simpleware-Segmentation experiment and simpleware_medical module#25

Merged
aylward merged 4 commits intomainfrom
simpleware_seg
Feb 7, 2026
Merged

ENH: Add Heart-Simpleware-Segmentation experiment and simpleware_medical module#25
aylward merged 4 commits intomainfrom
simpleware_seg

Conversation

@aylward
Copy link
Copy Markdown
Collaborator

@aylward aylward commented Feb 6, 2026

  • New experiment: Heart-Simpleware-Segmentation with demo notebook
  • Add segment_heart_simpleware.py and simpleware_medical package
  • Add SimplewareScript_heart_segmentation.py and tests
  • Update READMEs, pyproject.toml, and experiment tests

…cal module

- New experiment: Heart-Simpleware-Segmentation with demo notebook
- Add segment_heart_simpleware.py and simpleware_medical package
- Add SimplewareScript_heart_segmentation.py and tests
- Update READMEs, pyproject.toml, and experiment tests
Copilot AI review requested due to automatic review settings February 6, 2026 08:04
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds integration with Synopsys Simpleware Medical's ASCardio module for automated cardiac segmentation. The implementation provides a new experiment demonstrating heart segmentation through an external Simpleware Medical process.

Changes:

  • New SegmentHeartSimpleware class that inherits from SegmentChestBase and manages external Simpleware Medical process execution
  • New simpleware_medical package containing Python scripts for ASCardio integration
  • New Heart-Simpleware-Segmentation experiment with demonstration notebook
  • Test infrastructure updates to support prerequisite checking for dependent experiments
  • Configuration updates for mypy type checking

Reviewed changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated 18 comments.

Show a summary per file
File Description
src/physiomotion4d/segment_heart_simpleware.py Main segmentation class that spawns Simpleware Medical subprocess and processes results
src/physiomotion4d/simpleware_medical/__init__.py Package initialization for Simpleware Medical integration
src/physiomotion4d/simpleware_medical/SimplewareScript_heart_segmentation.py Python script executed within Simpleware Medical environment
src/physiomotion4d/simpleware_medical/test_physiomotion_heart_segmentation.py Development/test version of the Simpleware script
src/physiomotion4d/simpleware_medical/README.md Detailed documentation for the Simpleware integration
experiments/Heart-Simpleware-Segmentation/simpleware_heart_segmentation_demo.ipynb Demo notebook showing usage of the integration
experiments/Heart-Simpleware-Segmentation/README.md Experiment documentation and troubleshooting guide
experiments/Heart-Simpleware-Segmentation/.gitignore Experiment-specific gitignore patterns
tests/test_experiments.py Added prerequisite checking helper function for dependent experiments
pyproject.toml Added simpleware to mypy ignore list
.gitignore Added *.code-workspace pattern

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

self.simpleware_script_path,
"--exit-after-script",
"--no-splash",
tmp_input_image_file, # Input NIfTI file path as positional argument
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The command-line includes a positional argument with the input NIfTI file path (line 193), but this is placed after the --exit-after-script flag. Positional arguments for Simpleware typically need to come before flags, or they may be ignored or cause errors. Additionally, the script doesn't parse sys.argv to retrieve this value, so even if it's passed correctly, it wouldn't be used.

Copilot uses AI. Check for mistakes.
Comment on lines +14 to +20
### ⚠️ Current Status: Prototype

**This is currently a proof-of-concept implementation.**

Due to Simpleware Console mode limitations (cannot programmatically create documents), the current version produces **placeholder output**. The code demonstrates the intended API workflow and integration pattern.

See the Troubleshooting section below for details on this limitation and possible workarounds.
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a significant discrepancy between the two README files. The experiment README (lines 14-20) clearly states this is a "proof-of-concept" that produces "placeholder output" due to Simpleware Console mode limitations. However, the simpleware_medical README (line 21) claims "✅ FUNCTIONAL: This integration is ready for testing!" These conflicting status claims are misleading. Given the critical bugs identified in the implementation (missing image loading, incorrect executable path, etc.), the integration is not functional and the status documentation should be consistent.

Copilot uses AI. Check for mistakes.
Comment on lines +51 to +53
mask.MetaImageExport(
f"C:/src/Projects/PhysioMotion/physiomotion4d/experiments/mask{fixed_name}.mhd"
)
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hardcoded absolute path "C:/src/Projects/PhysioMotion/physiomotion4d/experiments/mask{fixed_name}.mhd" is problematic. This path is specific to a development environment and will fail when run on other systems. The code should use a relative path or accept the output directory as a parameter.

Copilot uses AI. Check for mistakes.
Comment on lines +36 to +47
mask_names = [
"Aorta",
"Left Ventricle",
"Right Ventricle",
"Left Atrium",
"Right Atrium",
"Myocardium",
"Left Coronary Artery",
"Right Coronary Artery",
"Pulmonary Artery",
]

Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The mask_names list is defined but never used. Instead, the code iterates over doc.GetMasks() directly on line 48, which means the list serves no purpose. If the intent is to validate or filter specific masks, this logic should be implemented. Otherwise, remove the unused variable to avoid confusion.

Suggested change
mask_names = [
"Aorta",
"Left Ventricle",
"Right Ventricle",
"Left Atrium",
"Right Atrium",
"Myocardium",
"Left Coronary Artery",
"Right Coronary Artery",
"Pulmonary Artery",
]

Copilot uses AI. Check for mistakes.

Example:
>>> segmenter.set_simpleware_executable_path(
... "C:/Program Files/Synopsys/Simpleware Medical/X-2025.06/SimplewareMedical.exe"
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docstring example on line 119 references "SimplewareMedical.exe" but should reference "ConsoleSimplewareMedical.exe" to be consistent with the intended console mode execution described in the documentation.

Suggested change
... "C:/Program Files/Synopsys/Simpleware Medical/X-2025.06/SimplewareMedical.exe"
... "C:/Program Files/Synopsys/Simpleware Medical/X-2025.06/ConsoleSimplewareMedical.exe"

Copilot uses AI. Check for mistakes.
Comment on lines +175 to +179
params_file = os.path.join(tmp_dir, "params.txt")
with open(params_file, "w") as f:
f.write(f"input_image:{tmp_input_image_file}\n")
f.write(f"output_directory:{tmp_dir}\n")

Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The segment_heart_simpleware.py creates a params.txt file with input_image and output_directory entries (lines 175-178), but the SimplewareScript_heart_segmentation.py script only reads the output_dir via app.GetInputValue() and never reads or parses the params.txt file. The input image path is written to params.txt but never retrieved, which means the script has no way to know which file to load.

Suggested change
params_file = os.path.join(tmp_dir, "params.txt")
with open(params_file, "w") as f:
f.write(f"input_image:{tmp_input_image_file}\n")
f.write(f"output_directory:{tmp_dir}\n")

Copilot uses AI. Check for mistakes.
Comment on lines +230 to +240
print(f"Class Output directory: {tmp_dir}")
sz = [s for s in preprocessed_image.GetLargestPossibleRegion().GetSize()]
sz = sz[::-1]
labelmap_array = np.zeros(sz, dtype=np.uint8)
for mask_id, mask_name in self.all_mask_ids.items():
output_file = os.path.join(tmp_dir, f"mask_{mask_name}.mhd")
if os.path.exists(output_file):
mask_image = itk.imread(output_file)
mask_array = itk.GetArrayFromImage(mask_image).astype(np.uint8)
if mask_id in mask_ids_of_interior_regions:
print(f"Class Mask ID: {mask_id}")
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Debug print statements (lines 230 and 240) should not be present in production code. Use the existing logging infrastructure (self.log_info or self.log_debug) instead to maintain consistency with the rest of the codebase and allow users to control logging verbosity.

Copilot uses AI. Check for mistakes.
self.set_other_and_all_mask_ids()

# Path to Simpleware Medical console executable
self.simpleware_exe_path = "C:/Program Files/Synopsys/Simpleware Medical/X-2025.06/SimplewareMedical.exe"
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The executable path uses "SimplewareMedical.exe" but the documentation and comments throughout the codebase (including the README) consistently reference "ConsoleSimplewareMedical.exe" as the required console version. This inconsistency will likely cause execution failures since the GUI version may not support the command-line flags being used.

Copilot uses AI. Check for mistakes.
" print(\"Results loaded successfully!\")\n",
" except (FileNotFoundError, OSError) as e:\n",
" print(f\"Error loading results: {e}\")\n",
" print(\"Set re_run_segmentation=True to generate new results.\")\n",
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error message on line 305 references a variable 're_run_segmentation' that doesn't exist in the notebook. This variable is mentioned in the README as a configuration parameter but is never actually defined or used in the notebook code. Either add this variable to the configuration section or update the error message to be accurate.

Suggested change
" print(\"Set re_run_segmentation=True to generate new results.\")\n",
" print(\"No existing results found. Run the segmentation cell to generate new results.\")\n",

Copilot uses AI. Check for mistakes.
The integration enables PhysioMotion4D to leverage Simpleware Medical's ASCardio module for automated cardiac segmentation. The implementation uses a two-component architecture:

1. **segment_heart_simpleware.py** (in parent directory): A Python class that inherits from `SegmentChestBase` and manages the external Simpleware Medical process
2. **physiomotion_heart_segmentation.py** (this directory): A Python script that runs within the Simpleware Medical environment and performs the actual segmentation using ASCardio
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The README documentation references a file named "physiomotion_heart_segmentation.py" multiple times (lines 10, 123, 185, 201, 233), but the actual file in the codebase is named "SimplewareScript_heart_segmentation.py". This inconsistency between documentation and actual filenames will cause confusion and potentially runtime errors if users follow the documentation.

Suggested change
2. **physiomotion_heart_segmentation.py** (this directory): A Python script that runs within the Simpleware Medical environment and performs the actual segmentation using ASCardio
2. **SimplewareScript_heart_segmentation.py** (this directory): A Python script that runs within the Simpleware Medical environment and performs the actual segmentation using ASCardio

Copilot uses AI. Check for mistakes.
- Fix experiment README: notebook name, label IDs (heart 1-6, vessels 7-10), workflow description
- Remove obsolete "placeholder output" limitation; state integration works as expected
- Update simpleware_medical README: --input-file/--input-value flow, structure IDs, example commands
- Correct segment_heart_simpleware docstring script name and result description
- Notebook: set label overlay vmax=10 to match actual labels
Copilot AI review requested due to automatic review settings February 6, 2026 20:32
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 12 changed files in this pull request and generated 10 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/test_experiments.py
Comment on lines +211 to +214
f"PCA model output directory not found: {pca_output_dir}. "
"Run the Heart-Create_Statistical_Model experiment first "
"(e.g. pytest tests/test_experiments.py::test_experiment_create_statistical_model -v -m experiment).",
)
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The skip instruction in this message suggests running the experiment test without the required --run-experiments flag. Since conftest.py auto-skips @pytest.mark.experiment tests unless that flag is provided, the suggested command will still skip and may confuse users. Update the example command to include --run-experiments (or reference the documented invocation pattern).

Copilot uses AI. Check for mistakes.
Comment thread tests/README.md Outdated
### Segmentation Tests (GPU Required)
- **`test_segment_chest_total_segmentator.py`** - TotalSegmentator chest CT segmentation
- **`test_segment_chest_vista_3d.py`** - NVIDIA VISTA-3D segmentation (requires 20GB+ RAM)
- **`test_segment_heart_simpleware.py`** - Simpleware Medical ASCardio heart segmentation (same data as experiments/Heart-Simpleware_Segmentation)
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This README lists tests/test_segment_heart_simpleware.py, but that test module does not exist in the repository (so the docs and suggested ignore command are currently inaccurate). Either add the missing test file or remove/adjust these README references to match the actual test suite.

Suggested change
- **`test_segment_heart_simpleware.py`** - Simpleware Medical ASCardio heart segmentation (same data as experiments/Heart-Simpleware_Segmentation)

Copilot uses AI. Check for mistakes.
Comment on lines +229 to +235
# Check if output file was created
sz = [s for s in preprocessed_image.GetLargestPossibleRegion().GetSize()]
sz = sz[::-1]
labelmap_array = np.zeros(sz, dtype=np.uint8)
interior_array = np.zeros(sz, dtype=np.uint8)
for mask_id, mask_name in self.all_mask_ids.items():
output_file = os.path.join(tmp_dir, f"mask_{mask_name}.mhd")
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

segmentation_method documents raising ValueError when no output segmentation is produced, but the implementation will silently return an all-zero labelmap if Simpleware exits successfully without writing any mask_*.mhd files. Add an explicit check after the subprocess run (e.g., verify at least one expected mask file exists / labelmap has any non-zero voxels) and raise a clear error when outputs are missing.

Copilot uses AI. Check for mistakes.
Comment on lines +102 to +104
# Path to Simpleware Medical console executable
self.simpleware_exe_path = "C:/Program Files/Synopsys/Simpleware Medical/X-2025.06/ConsoleSimplewareMedical.exe"

Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default simpleware_exe_path is hard-coded to a specific Windows install location/version. This makes the class non-portable by default and can break out-of-the-box on Windows machines with different versions/paths. Consider defaulting to an env var (e.g., SIMPLEWARE_CONSOLE_EXE), auto-detecting common install paths on Windows, and leaving it unset/None (with a clear error) on non-Windows platforms.

Copilot uses AI. Check for mistakes.
Comment on lines +60 to +62

# Control re-running
re_run_segmentation = True # Set False to load cached results
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This configuration snippet mentions re_run_segmentation / cached results, but the notebook does not define or use re_run_segmentation anywhere (so readers will expect caching behavior that doesn't exist). Either implement the caching toggle in the notebook or remove/adjust this documentation section.

Suggested change
# Control re-running
re_run_segmentation = True # Set False to load cached results

Copilot uses AI. Check for mistakes.

The class maintains specific ID mappings for:
- Heart structures (left/right atrium, left/right ventricle, myocardium)
- Major vessels (aorta, pulmonary artery, vena cava)
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The class docstring claims major vessels include "vena cava", but the implementation only maps aorta, pulmonary artery, and coronary arteries (IDs 7–10). Please update the docstring (or the ID mapping) so the documented structures match what the code actually produces.

Suggested change
- Major vessels (aorta, pulmonary artery, vena cava)
- Major vessels (aorta, pulmonary artery, coronary arteries)

Copilot uses AI. Check for mistakes.
Comment on lines +177 to +184
# Pass the input NIfTI file path directly as a command-line argument
# The script will receive it via sys.argv and also output path
# Use --run-script to execute the Python script
# Use --exit-after-script to close after execution
cmd = [
self.simpleware_exe_path,
"--input-file", # Use only with ConsoleSimplewareMedical.exe
tmp_input_image_file, # Input NIfTI file path as positional argument
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These comments say the script will receive the input path via sys.argv, but SimplewareScript_heart_segmentation.py does not read sys.argv—it relies on Simpleware loading the document via --input-file and uses app.GetInputValue() for the output directory. Please update the comments to reflect the actual communication mechanism to avoid misleading future maintenance.

Suggested change
# Pass the input NIfTI file path directly as a command-line argument
# The script will receive it via sys.argv and also output path
# Use --run-script to execute the Python script
# Use --exit-after-script to close after execution
cmd = [
self.simpleware_exe_path,
"--input-file", # Use only with ConsoleSimplewareMedical.exe
tmp_input_image_file, # Input NIfTI file path as positional argument
# The input NIfTI file is loaded by Simpleware via the --input-file option.
# The temporary directory for output masks is passed via --input-value and
# is retrieved inside SimplewareScript_heart_segmentation.py using
# app.GetInputValue().
# Use --run-script to execute the Python script and --exit-after-script to
# close Simpleware after the script finishes.
cmd = [
self.simpleware_exe_path,
"--input-file", # Use only with ConsoleSimplewareMedical.exe
tmp_input_image_file,

Copilot uses AI. Check for mistakes.
Comment on lines +170 to +190
### Modifying ASCardio Parameters

To customize ASCardio segmentation parameters, edit `SimplewareScript_heart_segmentation.py`:

```python
cardio.auto_segment(
image=image,
segment_chambers=True,
segment_myocardium=True,
segment_vessels=True,
# Add custom parameters here
)
```

### Adding Custom Structures

To segment additional structures:

1. Update the `mask_id_mapping` dictionary in `SimplewareScript_heart_segmentation.py`
2. Update `heart_mask_ids` or `major_vessels_mask_ids` in `segment_heart_simpleware.py`

Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This README suggests customizing ASCardio via cardio.auto_segment(...) and updating a mask_id_mapping in SimplewareScript_heart_segmentation.py, but the current script uses CalculateHeartCTRegionOfInterest/ApplyHeartCTTool and has no mask_id_mapping. Update the customization instructions to match the actual Simpleware API calls used here (or add the referenced mapping/entry points if intended).

Copilot uses AI. Check for mistakes.
"The `SegmentHeartSimpleware` class provides:\n",
"- Automated heart chamber segmentation (LV, RV, LA, RA)\n",
"- Myocardium segmentation\n",
"- Major vessel segmentation (aorta, pulmonary artery, vena cava)\n",
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The notebook intro claims major vessel segmentation includes the "vena cava", but SegmentHeartSimpleware currently maps major vessels to aorta, pulmonary artery, and coronary arteries (IDs 7–10). Please update the notebook text (or the implementation) so the documented outputs match the actual label set.

Suggested change
"- Major vessel segmentation (aorta, pulmonary artery, vena cava)\n",
"- Major vessel segmentation (aorta, pulmonary artery, coronary arteries)\n",

Copilot uses AI. Check for mistakes.
"# Optional: Set custom Simpleware path if not in default location\n",
"custom_simpleware_path = None\n",
"# Example:\n",
"# custom_simpleware_path = \"D:/Synopsys/Simpleware/SimplewareMedical.exe\"\n",
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The example custom path points to SimplewareMedical.exe (GUI), but SegmentHeartSimpleware expects the console executable (ConsoleSimplewareMedical.exe) and passes console-only flags like --input-file/--no-progress. Update the example path to the console executable to prevent users from copying a non-working command.

Suggested change
"# custom_simpleware_path = \"D:/Synopsys/Simpleware/SimplewareMedical.exe\"\n",
"# custom_simpleware_path = \"D:/Synopsys/Simpleware/ConsoleSimplewareMedical.exe\"\n",

Copilot uses AI. Check for mistakes.
@aylward aylward merged commit 2e4f6a6 into main Feb 7, 2026
11 checks passed
@aylward aylward deleted the simpleware_seg branch February 7, 2026 23:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants